Number widget validation can break AJAX actions

Just tested this with the latest code for both core and entity_browser and I can't reproduce the issue, the modal opens correctly. Can you please test again with a clean Drupal install?

I have a similar test case which I wanted to share. The patch on this issue seems to solve the problem for me.

I have a custom entity with some fields on it, one of them being an inline entity form and another being number (required integer).

If I provide a value in my number field it then allows me to use my inline entity reference field without any errors or warnings.

My contrib modules are as follows:
- adminimal_theme
- commerce
- features
- inline_entity_form

@swental I am new to the release process for issues. How do we go about running tests? I would like to help out where possible!

@colinstillwell88

Well, what we need is a test only patch that proves the bug. We can add a test in NumberFieldTest for it.
We'll also have to double check we don't introduce regressions with the original patch though, so we might have to check other tests or even write more.

We'll also have to update the Issue Summary because the bug is not only about dialogs or so, it's about a required number and any ajax request after that.

I'll probably post a patch tonight already because this is holding me up as well. After I post the patch, we'll need reviewers, people who look at the code, but also confirm it actually works, although we have some confirmations already here and from #2648520: Recoverable fatal error: Argument 1 passed to Drupal\Core\Form\FormState::setError() must be of the type array, null given.

Here's a test only patch, will fail horribly.

I still can't reproduce this on my local, even after following the steps from the test in #9 :(

@amateescu does the test fail on your local machine ?
Maybe PHP version ? Haven't tested myself on PHP 7, will try that today.

Yep, the test fails and I tried to look into it a bit with debug() calls but I won't get anywhere without xdebug.

What I was able to find is only that in \Drupal\Core\Field\WidgetBase::flagErrors(), line 423, $violation->getPropertyPath() returns an empty string, which means that $delta_element = $element; below will be $element[0]['value'] instead of just $element['value'], and that's why the call to $this->errorElement() below will send the wrong form array structure.

And look who we have here: #2553983: Required textarea with summary breaks ajax events for other fields.

Wondering if we shouldn't fix this more upstream, it seems like more widgets can trigger this bug.

Maybe in #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements ? (although I haven't fully read the patch though and yched has won't fixed it)

Yep, the @todo from the patch in #2553983-3: Required textarea with summary breaks ajax events for other fields. clearly shows that we need to fix the violation property paths :)

Hi!
I got the same error when adding a new node in the entity reference field with inline entity form module.

Notice: Undefined index: value в Drupal\Core\Field\Plugin\Field\FieldWidget\NumberWidget->errorElement() (line 119 file .../core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/NumberWidget.php).

Recoverable fatal error: Argument 1 passed to Drupal\Core\Form\FormState::setError() must be of the type array, null given, called in .../core/lib/Drupal/Core/Field/WidgetBase.php on line 446 and defined in Drupal\Core\Form\FormState->setError() (line 1156 file .../core/lib/Drupal/Core/Form/FormState.php).

The patch from #2 @jfrederick helped me! Thank you so much!

I have the same problem with custom validation.
I think it is better to remove errorElement method, as its parent (WidgetBase) does exactly what patch of #2 does.

#19 is a good workaround until this is fixed upstream.

Since this involves unpredictable fatal errors for end users after the site builder assembles a fully supported data model in the UI alone for a common usecase, that is a strong case for this issue to be considered as a major.

#2619878-2: Recoverable fatal error: Argument 1 passed to Drupal\\Core\\Form\\ FormState::setError() must be of the type array uses a different approach than the patch in #19.
I feel that a combined patch would do the trick.

A combined patch is not possible here. The two approaches are mutually exclusive.

One edits the problematic line of code and the other approach deletes it to allow the class being extended to handle this function.

The later approach keeps us from repeating code.

Ok, here is a patch with the test of swentel (#9)

Thanks for reviewing

I had the same problem with an entitybrowser on an entity reference field, once I added number fields, they stopped working. Patch 25 fixed the issue.

We have been using #25 on several builds and it fixes the issue.
Includes the test from #9 and it is green.

I think this is RTBC

Shouldn't we be pushing for a generic fix for this... see #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements.

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/NumberWidget.php
@@ -107,11 +107,4 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
-  /**
-   * {@inheritdoc}
-   */
-  public function errorElement(array $element, ConstraintViolationInterface $violation, array $form, FormStateInterface $form_state) {
-    return $element['value'];
-  }

I'm not sure that removing this is correct - when the violation is due to the numeric field it should be set on the value element.

I think the correct thing to do here is work on #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements and then use this issue to add the missing test coverage if that one does not.

Yep reading the issue summary of #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements I'm convinced that we should attempt the generic fix and that the fix here is wrong. Postponing on that issue.

Patch #25 was failing for me on 8.1.8. Same patch against current 8.1.x for those of us using this in production sites, until the generic fix is finished.

UPDATED: Now I think it's just a composer issue on my system. This patch is not necessary.

subscribing

The core committers and entity and field system maintainers discussed this issue awhile back and agreed it is a major bug because it is a fatal error triggerable through the user interface. It is currently blocked on #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements. It may turn out to be a duplicate of that issue, but we should confirm once that issue is resolved.

Attaching fix for Drupal 8.3.1

@nettantra please read the comments in #29 and #35 this issue is postponed on #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements - it would be great if people can work on getting the generic issue fixed. Thanks.

#36 Works for me with drupal 8.3.4

The patch #36 work for me so with drupal 8.3.4.

For those of us who need a temporary fix, here is a patch that applies to 8.3.5.

patch without extra test as temporary fix against 8.3.5

We now have a dedicated issue for handling this problem in a generic way: #2901943: Content entity form validation does not respect the #limit_validation_errors property from field widgets

Hello all,

@amateescu, we still get this error on our Drupal installation (8.3.7) with your patch on the other issue #2901943.

Can someone confirm that?

@traedamatic: This issue was happening to us too, but the patch in #2901943 fixed it.

Temporary fix until Drupal core team release the full docs of errorElement.

FIX ISSUES w/ Paragraphs project.
FIX ISSUES w/ ajax calls and Number Widgets.

I do not did too many tests, so use carefully.

DRUPAL VERSION: 8.3.7

@bachilli, the proper fix for this bug has already been committed to 8.4.x and should be available in 8.4.1.

@amateescu can you link the commit in this post? thanks!

@bachilli, sure, the patch that was committed is the one from #2901943-18: Content entity form validation does not respect the #limit_validation_errors property from field widgets and the actual commit for 8.4.x is http://cgit.drupalcode.org/drupal/commit/?id=a9375d3

@amateescu nice! but I've already tried this solution and conflicts with Paragraphs but I dont know why.

@bachilli, you might also want to try another patch that's fixing stuff in the same area: #2784015-22: Widget validation crashes on ItemList violations for widgets with a custom errorElement() implementation

After updating to D8.4.2 (where patch from #2901943 is included), I still had the same error on a required number field.

But after applying the patch from #2784015 (see comment above), the error was gone. So I guess we should add #2784015 as related issue here.